Use Base Class Rule #84

masamichiueta · 2019-03-11T09:54:43Z

This PR is a proposal of a new rule, UseBaseClass.

What is `UseBaseClass` rule?

UseBaseClass rule is a rule to check whether an UI element uses custom class which is set as base class in config.

For example, if we have some base classes of UILabel like PrimaryLabel and SecondaryLabel that should be applied to all labels through the project, this rule checks whether labels in storyboards and xibs are set to base classes.

Why is this rule required?

As I describe above, many apps commonly have their own base UI components. But we often forget to set the custom class to the base classes in storyboard or xib. And new members often overlook the base components.

This rule can prevent the problem and this is the reason why I developed this rule.

How does it work?

Set element_class and base_classes in config like

use_base_class_rule:
  - element_class: UILabel
    base_classes:
      - PrimaryLabel
      - SecondaryLabel

This enables to check UILabel uses PrimaryLabel or SecondaryLabel. If UILabel does not use the base classes, Xcode shows warnings like below.

kateinoigakukun

Thank you for your contribution 🎉
Looks almost OK to me. But this rule can't work for inherited class because IB doesn't know about Swift type information.
So please add that caution on rule description of README.

kateinoigakukun · 2019-03-11T12:03:10Z

Sources/IBLinterKit/Rules/UseBaseClassRule.swift

+            return views.flatMap { validate(for: $0.view, file: xib, fileNameWithoutExtension: xib.fileNameWithoutExtension) }
+        }
+
+        private func validate<T: InterfaceBuilderFile>(for view: ViewProtocol, file: T, fileNameWithoutExtension: String) -> [Violation] {


Is fileNameWithoutExtension needed?

Oh, it's not used anywhere😅 I can remove it!

kateinoigakukun · 2019-03-11T12:10:59Z

Sources/IBLinterKit/UseBaseClassConfig.swift

+        case baseClasses = "base_classes"
+    }
+
+    public static let `default` = UseBaseClassConfig.init()


I think this default and private init is not used anywhere. Can you remove them?

I see👍 Thanks!

kateinoigakukun · 2019-03-11T12:20:15Z

Sources/IBLinterKit/UseBaseClassConfig.swift

+
+    public init(from decoder: Decoder) throws {
+        let container = try decoder.container(keyedBy: CodingKeys.self)
+        elementClass = try container.decodeIfPresent(Optional<String>.self, forKey: .elementClass).flatMap { $0 } ?? ""


If elementClass is nil or not present, this should throw a decoding error.
But in the case of baseClasses, it's OK because empty array in YAML is expressed in the same way of nil

elementClass: # This is invalid. Shouldn't be nil baseClasses: # It's OK. This means empty array

I'll fix to throw an error!

masamichiueta · 2019-03-11T13:59:24Z

@kateinoigakukun Thank you for your quick review!

this rule can't work for inherited class because IB doesn't know about Swift type information

[Q] What do you mean about inherited class?

kateinoigakukun · 2019-03-11T14:43:04Z

@masamichiueta Oh, I misunderstood 😅. I mean PrimaryLabel can be inherited.
Do you think that CustomizedPrimaryLabel should be invalid? If so, it's my misunderstanding and you don't need to change README

class PrimaryLabel: UILabel {
    ...
}

class CustomizedPrimaryLabel: PrimaryLabel {
    ... 
}

masamichiueta · 2019-03-12T00:56:48Z

I understand! As you said, this rule does not work for inherited classes. A developer needs to add the inherited classes to base_classes in yml.

I add caution on rule description👍

masamichiueta · 2019-03-12T01:57:36Z

Updated. Please review again🙏

kateinoigakukun

OK, LGTM!

masamichiueta added 7 commits March 11, 2019 15:02

add UseBaseClassRule

aaa2453

add config and validation

2695d06

add UseBaseClassRule to allRules

e9227f0

rename config

6020ec7

update message

b55853e

rename variable name

7497310

update readme

2aa3a3a

kateinoigakukun self-requested a review March 11, 2019 11:57

kateinoigakukun approved these changes Mar 11, 2019

View reviewed changes

masamichiueta added 4 commits March 12, 2019 10:04

fix review comment

82ad1bd

update readme

177fcd6

remove footernote style

0e54e0d

fix config file

0517f92

kateinoigakukun approved these changes Mar 12, 2019

View reviewed changes

kateinoigakukun merged commit 316dea3 into IBDecodable:master Mar 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Base Class Rule #84

Use Base Class Rule #84

masamichiueta commented Mar 11, 2019

kateinoigakukun left a comment •

edited

kateinoigakukun Mar 11, 2019

masamichiueta Mar 11, 2019

kateinoigakukun Mar 11, 2019

masamichiueta Mar 11, 2019

kateinoigakukun Mar 11, 2019

masamichiueta Mar 11, 2019

masamichiueta commented Mar 11, 2019 •

edited

kateinoigakukun commented Mar 11, 2019 •

edited

masamichiueta commented Mar 12, 2019

masamichiueta commented Mar 12, 2019

kateinoigakukun left a comment

Use Base Class Rule #84

Use Base Class Rule #84

Conversation

masamichiueta commented Mar 11, 2019

What is UseBaseClass rule?

Why is this rule required?

How does it work?

kateinoigakukun left a comment • edited

Choose a reason for hiding this comment

kateinoigakukun Mar 11, 2019

Choose a reason for hiding this comment

masamichiueta Mar 11, 2019

Choose a reason for hiding this comment

kateinoigakukun Mar 11, 2019

Choose a reason for hiding this comment

masamichiueta Mar 11, 2019

Choose a reason for hiding this comment

kateinoigakukun Mar 11, 2019

Choose a reason for hiding this comment

masamichiueta Mar 11, 2019

Choose a reason for hiding this comment

masamichiueta commented Mar 11, 2019 • edited

kateinoigakukun commented Mar 11, 2019 • edited

masamichiueta commented Mar 12, 2019

masamichiueta commented Mar 12, 2019

kateinoigakukun left a comment

Choose a reason for hiding this comment

What is `UseBaseClass` rule?

kateinoigakukun left a comment •

edited

masamichiueta commented Mar 11, 2019 •

edited

kateinoigakukun commented Mar 11, 2019 •

edited